Skip to content

Fix audio/video desync caused by floating point cumulative errors#3837

Merged
TobiGr merged 1 commit intoTeamNewPipe:devfrom
Stypox:audio-sync
Jul 2, 2020
Merged

Fix audio/video desync caused by floating point cumulative errors#3837
TobiGr merged 1 commit intoTeamNewPipe:devfrom
Stypox:audio-sync

Conversation

@Stypox
Copy link
Member

@Stypox Stypox commented Jun 30, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

After doing a git bisect I found out which commit caused the desync to appear: 3855e48
The problem was that the value of playback speed, even though displayed as 1.0x, was instead 1.00267911. This was probably caused by cumulative floating point errors after saving and restoring the value many times from shared preferences. This PR rounds speed and pitch to 2 decimal places before saving them or feeding them to the player, so that any floating point error is eliminated and the issue is not there anymore.

Fixes the following issue(s)

Testing apk

The reason as to why having a playback speed very near 1.0 caused desync is still unknown to me, but I tested this PR with the settings provided at #3550 (comment) and the issue is not there anymore (I am sure it was there before with the same setup).
@LukasThyWalls @juantxorena @ajtpak @mgolden356 @Godecki @macearl @vadiof @loque036 @B0pol @coolsan06 @theScrabi could you test this apk and confirm it fixes the issue?
app-debug.zip

Agreement

@Stypox Stypox added ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug player Issues related to any player (main, popup and background) labels Jun 30, 2020
@Stypox Stypox mentioned this pull request Jun 30, 2020
@mgolden356
Copy link

mgolden356 commented Jun 30, 2020

This works on my end so far, after a ten minute video. Due to the nature of the variability of when I get desyncs, I may be updating this in the future. Thank you! This was my number one pressing concern with NewPipe.

Works so far after a 2 hour video at 1x speed at 720p. That's pretty good by any stretch of the imagination. Will comment further.

This makes a lot of sense. I always export and import my data and settings. And this must be why at first the desync takes a while to show up, but then it starts being there all the time/constant. The 1.01x, the not importing settings, everything.

If it's relevant I have a Moto g7 power Android 9 latest security.

@TobiGr TobiGr added this to the 0.19.6 milestone Jun 30, 2020
@TobiGr TobiGr merged commit 9516d9d into TeamNewPipe:dev Jul 2, 2020
@theScrabi
Copy link
Member

Yop works like a charm on my aquaris X lineage 16.1

@coolsan06
Copy link

It works really well with Android 9 in a v30.

This was referenced Jul 8, 2020
@Stypox Stypox deleted the audio-sync branch August 4, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug player Issues related to any player (main, popup and background)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audio/Video desync

5 participants